[Merged by Bors] - Add self-hosted runners v2 #4506

paulhauner · 2023-07-14T02:50:28Z

Issue Addressed

NA

Proposed Changes

Carries on from #4115, with the following modifications:

Self-hosted runners are only enabled if github.repository == sigp/lighthouse.
- This allows forks to still have Github-hosted CI.
- This gives us a method to switch back to Github-runners if we have extended downtime on self-hosted.
Does not remove any existing dependency builds for Github-hosted runners (e.g., installing the latest Rust).
Adds the WATCH_HOST environment variable which defines where we expect to find the postgres db in the watch tests. This should be set to host.docker.internal for the tests to pass on self-hosted runners.

Additional Info

NA

.github/workflows/test-suite.yml

paulhauner · 2023-07-14T06:34:27Z

A list of things that are different between Github-runners and our self-hosted runners for future reference:

Self-hosted runners don't have sudo.
Self-hosted runners fail rustup update with error: could not rename component file from '/opt/rust/toolchains/stable-x86_64-unknown-linux-gnu/etc/bash_completion.d' to '/opt/rust/tmp/bsq3a_35gvhrh01r_dir/bk': Invalid cross-device link (os error 18) (failed CI run)

paulhauner · 2023-07-14T23:35:20Z

We're now using github.repository == 'sigp/lighthouse' to enable/disable self-hosted runners. I originally wanted to use a repository variable for this purpose, however they "... are not passed to workflows that are triggered by a pull request from a fork.". This means that a PR from paulhauner/lighthouse to sigp/lighthouse wouldn't use self-hosted runners.

I would have liked to set an env variable at the top of the play that could have been used to de-duplicate some of the runs_on values. But alas, using env variables in runs_on is not supported (relevant issue actions/runner#480).

This reverts commit 4749645.

antondlr · 2023-07-17T06:06:00Z

if these additions differ from the GitHub-hosted runners (i.e., install 7zip), I'll add them to the runner image.
ideally, we keep the self-hosted and GitHub-hosted runners hot-swappable.

I'll document where and how to update the images Real Soon™️, too.

jimmygchen · 2023-07-20T01:13:36Z

.github/workflows/test-suite.yml

+    - name: Install apt dependencies
+      if: env.SELF_HOSTED_RUNNERS
+      run: |
+          sudo apt-get update
+          sudo apt-get install libpq-dev  # Required for Postgres in ./watch.


we could probably remove this once this is baked into the self hosted runner image

.github/workflows/test-suite.yml

paulhauner · 2023-07-21T01:49:57Z

This is ready for review! 🎉

jimmygchen · 2023-07-21T02:00:06Z

.github/workflows/test-suite.yml

@@ -61,14 +65,15 @@ jobs:
    - name: Install Foundry (anvil)
      uses: foundry-rs/foundry-toolchain@v1
    - name: Run tests in release
-      run: make test-release
+      run: WATCH_HOST=host.docker.internal make test-release


I think this may not work on GitHub hosted runners if they're not running in docker containers, perhaps we can leave it empty if env.SELF_HOSTED_RUNNERS == false so it defaults to localhost?

Good call, I had a shot at fixing this in ce4ed6e.

jimmygchen · 2023-07-21T02:01:29Z

.github/workflows/test-suite.yml

@@ -143,7 +153,7 @@ jobs:
    - name: Install Foundry (anvil)
      uses: foundry-rs/foundry-toolchain@v1
    - name: Run tests in debug
-      run: make test-debug
+      run: WATCH_HOST=host.docker.internal make test-debug


same as above

jimmygchen · 2023-07-21T02:07:40Z

.github/workflows/test-suite.yml

@@ -83,6 +88,7 @@ jobs:
    - name: Install make
      run: choco install -y make
    - uses: KyleMayes/install-llvm-action@v1
+      if: env.SELF_HOSTED_RUNNERS == false


I think we're good with this but just something worth thinking - Is there any benefit of leaving these conditionals out?

There are a few benefits:

CI be always the same dep versions on self-hosted / github-hosted runners

For Rust, the workflow will automatically update to latest version as soon as it's released, it could take an extra 15s or more to install. But then we'll know we need to update our runner image.

Downside:

it could be a bit slower, but AFAIK rustup update stable just returns immediately if we're already on the same version.

IIRC the self-hosted runners fail if we run that llvm install action.

Same with the Rust update ones.

ahh privileges! happy to leave it as it is 👍

IMO it would be ideal if we can have the images perform exactly the same. Perhaps we can make an issue to come back for it.

I suspect that we're going to eventually drift into the territory of Github-hosted runners failing, though. If we're not actually testing on the Github-hosted ones they're bound to bit-rot.

Yeah, that's a good point - I'm hesitant to make the workflow more complicated, but if we really want to run GitHub-hosted runners, we could still run them on PRs, and use the self-hosted ones on staging 😛 , but perhaps something to think about later down the line.

jimmygchen

LGTM! Woot, excited to have this merged 🔥 🚀 🎉 ⏩ ⏩ ⏩

paulhauner · 2023-07-21T02:16:08Z

I'll merge this when CI passes 🎉

paulhauner · 2023-07-21T04:46:28Z

Yay, CI passed 🎉

bors r+

## Issue Addressed NA ## Proposed Changes Carries on from #4115, with the following modifications: 1. Self-hosted runners are only enabled if `github.repository == sigp/lighthouse`. - This allows forks to still have Github-hosted CI. - This gives us a method to switch back to Github-runners if we have extended downtime on self-hosted. 1. Does not remove any existing dependency builds for Github-hosted runners (e.g., installing the latest Rust). 1. Adds the `WATCH_HOST` environment variable which defines where we expect to find the postgres db in the `watch` tests. This should be set to `host.docker.internal` for the tests to pass on self-hosted runners. ## Additional Info NA Co-authored-by: antondlr <[email protected]>

bors · 2023-07-21T05:29:57Z

Pull request successfully merged into unstable.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

## Issue Addressed NA ## Proposed Changes Carries on from sigp#4115, with the following modifications: 1. Self-hosted runners are only enabled if `github.repository == sigp/lighthouse`. - This allows forks to still have Github-hosted CI. - This gives us a method to switch back to Github-runners if we have extended downtime on self-hosted. 1. Does not remove any existing dependency builds for Github-hosted runners (e.g., installing the latest Rust). 1. Adds the `WATCH_HOST` environment variable which defines where we expect to find the postgres db in the `watch` tests. This should be set to `host.docker.internal` for the tests to pass on self-hosted runners. ## Additional Info NA Co-authored-by: antondlr <[email protected]>

antondlr and others added 4 commits March 20, 2023 11:32

run bigger tasks on self hosted runners

6e639fe

Merge branch 'unstable' into self-hosted-runners

769b130

Revert dependency installation

da921b0

Add libpq-dev

2eb7457

paulhauner changed the base branch from stable to unstable July 14, 2023 02:50

paulhauner added 7 commits July 14, 2023 12:50

Empty commit

42ca4c8

Run debug tests on self-hosted runners

9a4c459

Remove Rust update for self-hosted tasks

d369c18

Revert to Github runner for Windows tests

eba6dd3

Add runners as variables

9e5e0f2

Remove sudo when installing libpq-dev

8bc4e99

Run rustup update when not using self-hosted

3472e60

jimmygchen reviewed Jul 14, 2023

View reviewed changes

.github/workflows/test-suite.yml Outdated Show resolved Hide resolved

paulhauner added 2 commits July 14, 2023 16:20

Use repository-level environment variable

e705411

Remove bangs

853d447

paulhauner commented Jul 14, 2023

View reviewed changes

.github/workflows/test-suite.yml Show resolved Hide resolved

paulhauner added 6 commits July 14, 2023 17:28

Use == 'true' in ternary operator

d85891e

Add temporary debug stuff

bc883e6

Add another debug line

a90df60

Modify debug statements

7148b13

More debugging

04deb0f

Start using github.repository

5880179

paulhauner added 6 commits July 15, 2023 09:44

Try using toJson

4749645

Revert "Try using toJson"

1cd9e9d

This reverts commit 4749645.

Test with just string for runs-on

74542b7

Try fromJson

c3625d3

Implement fromJson globally

7db66c7

Switch windows to self-hosted

e008422

paulhauner added 3 commits July 17, 2023 09:05

Add 7zip to windows runner

a0619ac

Install 7zip rather than 7zip.install

b57c114

Disable LLVM update for Windows runner

c9476bd

Merge branch 'unstable' into self-hosted-runners-paul

7f45652

jimmygchen reviewed Jul 20, 2023

View reviewed changes

.github/workflows/test-suite.yml Outdated Show resolved Hide resolved

paulhauner added 4 commits July 20, 2023 18:11

Change watch DB host

2f9243f

Add watch host for debug tests too

0adaf83

Fix compile error

21be30c

Remove libpq install

acb9c29

paulhauner mentioned this pull request Jul 21, 2023

Introduce self-hosted runners #4115

Closed

paulhauner added the ready-for-review The code is ready for review label Jul 21, 2023

jimmygchen reviewed Jul 21, 2023

View reviewed changes

Allow WATCH_HOST to be localhost on Github runners

ce4ed6e

jimmygchen reviewed Jul 21, 2023

View reviewed changes

jimmygchen approved these changes Jul 21, 2023

View reviewed changes

jimmygchen added infra-ci ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Jul 21, 2023

bors bot changed the title ~~Add self-hosted runners v2~~ [Merged by Bors] - Add self-hosted runners v2 Jul 21, 2023

bors bot closed this Jul 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Add self-hosted runners v2 #4506

[Merged by Bors] - Add self-hosted runners v2 #4506

paulhauner commented Jul 14, 2023 •

edited

Loading

paulhauner commented Jul 14, 2023

paulhauner commented Jul 14, 2023

antondlr commented Jul 17, 2023

jimmygchen Jul 20, 2023

paulhauner Jul 21, 2023

paulhauner commented Jul 21, 2023

jimmygchen Jul 21, 2023

paulhauner Jul 21, 2023

jimmygchen Jul 21, 2023

jimmygchen Jul 21, 2023

paulhauner Jul 21, 2023

paulhauner Jul 21, 2023

jimmygchen Jul 21, 2023

paulhauner Jul 21, 2023

jimmygchen Jul 21, 2023

jimmygchen left a comment

paulhauner commented Jul 21, 2023

paulhauner commented Jul 21, 2023

bors bot commented Jul 21, 2023

[Merged by Bors] - Add self-hosted runners v2 #4506

[Merged by Bors] - Add self-hosted runners v2 #4506

Conversation

paulhauner commented Jul 14, 2023 • edited Loading

Issue Addressed

Proposed Changes

Additional Info

paulhauner commented Jul 14, 2023

paulhauner commented Jul 14, 2023

antondlr commented Jul 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulhauner commented Jul 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jimmygchen left a comment

Choose a reason for hiding this comment

paulhauner commented Jul 21, 2023

paulhauner commented Jul 21, 2023

bors bot commented Jul 21, 2023

paulhauner commented Jul 14, 2023 •

edited

Loading